added the 5 functions to test#10

Open

davisgd wants to merge 2 commits intoAmericaCampaign:masterfrom

davisgd:data-and-functions-1

davisgd commented Aug 23, 2017

No description provided.

davisgd added 2 commits

August 23, 2017 10:51


          added the 5 functions to test

00f0839


          added getOrderInfo

de11260

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getProductById.js

+              const getProductById = (id) => {
+                let product
+                for (let n = 0; n < DATA.products.length; n += 1) {

Contributor

jcheroske Aug 23, 2017

It's standard practice to use the variable name i in a for loop.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getProductById.js

		@@ -0,0 +1,13 @@
		import DATA from '../DATA'

		const getProductById = (id) => {

Contributor

jcheroske Aug 23, 2017 •

edited

Loading

The expected function signature is:

const getProductById = (data, productId) => {

Contributor

jcheroske Aug 23, 2017

You also need to test to make sure data, data.products, and productId are not null.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getProductById.js

		@@ -0,0 +1,13 @@
		import DATA from '../DATA'

Contributor

jcheroske Aug 23, 2017

You don't need to import the data, since the test will pass it in as the first parameter.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getUserById.js

+                  return null
+                } else {
+                  DATA.users.forEach((u) => {
+                    if (u.userId === DATA.id) {

Contributor

jcheroske Aug 23, 2017

You don't want to test against DATA.id. What is the correct comparison?

if (u.userId === ???) { // <-- What do we really want to compare `u.userId` against?

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getActiveUsers.js

		@@ -0,0 +1,13 @@
		import DATA from '../DATA'

Contributor

jcheroske Aug 23, 2017

You don't need to pass in the data. The first parameter of all of your functions should be the data.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getActiveUsers.js

		@@ -0,0 +1,13 @@
		import DATA from '../DATA'

		const getActiveUsers = (id) => {

Contributor

jcheroske Aug 23, 2017

Add a data parameter as the first parameter.

Contributor

jcheroske Aug 23, 2017

Sanity check the parameters according to the getActiveUsers.test.js file.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

		@@ -0,0 +1,17 @@
		import DATA from '../DATA'

Contributor

jcheroske Aug 23, 2017

Remove the import.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

+              import DATA from '../DATA'
+              const getMostExpensiveProduct = (data) => {
+                if (data == null) {

Contributor

jcheroske Aug 23, 2017

This check is correct, but you need one more I think.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

+                if (data == null) {
+                  return null
+                }
+                let mostExpensiveProduct = 0

Contributor

jcheroske Aug 23, 2017

There are many ways to solve this one. You need to declare a variable like you did, but let's not initialize it: let mostExpensiveProduct

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

+                  return null
+                }
+                let mostExpensiveProduct = 0
+                DATA.products.forEach((m) => {

Contributor

jcheroske Aug 23, 2017

The forEach loop passes in the actual object as the first param to your function. Instead of calling it m, call it currentProduct

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

+                }
+                let mostExpensiveProduct = 0
+                DATA.products.forEach((m) => {
+                  const currentProduct = data.products[m]

Contributor

jcheroske Aug 23, 2017

Delete this line.

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getMostExpensiveProduct.js

+                let mostExpensiveProduct = 0
+                DATA.products.forEach((m) => {
+                  const currentProduct = data.products[m]
+                  if (currentProduct.price > mostExpensiveProduct) {

Contributor

jcheroske Aug 23, 2017

Two things need to change here:

You should test to see if mostExpensiveProduct == null. OR:
Compare the prices of the currentProduct and the mostExpensiveProduct

If either of those are true, enter your if

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getOrderInfo.js

+                  return null
+                }
+                const orderArray = []
+                DATA.orders.forEach((o) => {

Contributor

jcheroske Aug 23, 2017

Replace the o parameter with currentOrder

jcheroske reviewed

View reviewed changes

data-and-functions-1/src/functions/getOrderInfo.js

+                }
+                const orderArray = []
+                DATA.orders.forEach((o) => {
+                  const currentOrder = DATA.orders[o]

Contributor

jcheroske Aug 23, 2017

Delete this

Contributor

jcheroske commented Aug 23, 2017

I added lots of comments to your code! Take a look. If you're confused in any way, please reach out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet